Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MINIFICPP-2346: Build MiNiFi Core, MainExe, Standard-Processors with Conan2 #1775

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

james94
Copy link
Contributor

@james94 james94 commented May 2, 2024

Previous Title: MINIFICPP-2346: Replace CMake FetchContent with Conan Packager for Faster Builds

NOTE: I do want to give a heads up, this is a draft PR. I will make further progress soon.

P2: Verifying Can Build MiNiFi Extensions & GTESTs with Conan CMake

UPDATE (May 13, 2024) - 22205c9: Fetch and Pulled latest upstream main branch of parent MiNiFi to my fork main branch of MiNiFi repo; thus, merged in the latest updates from main branch of parent MiNiFi to my MINIFICPP-2346 branch. Resolved conflicts between a few CMake files coming from Date.cmake and BundledRocksdb.cmake files. For standalone CMake build appraoch, ran GTESTs with 100% (238/238) tests passing. I did the conan build approach and ran GTESTs with 99% (237/238) passing.

UPDATE (May 13, 2024) - 59ffefb: Enabled LIBRDKAFKA & GRPC_FOR_LOKI MiNiFi C++ extension using Conan & CMake, Improved Running Python Maven Executable for JNI & SFTP Test Server for conan approach and Improved standalone CMake approach for building PcapPlusPlus with its dependency on LibPcap. LIBRDKAFKA required creating a librdkafka conan package passing similar CMake definitions to it to build it like we did for standalone CMake approach. GRPC_FOR_LOKI required creating a grpc conan package that uses minifi's openssl and libsystemd conan packages. In both LIBRDKAFKA & GRPC_FOR_LOKI, I updated their .cmake Bundles to account for the condition of using their conan package, so minimal changes would be needed to their extension CMakeLists.txt file. In LIBRDKAFKA's extension CMakeLists.txt file, it didnt really change after integrating the conan package since I created a library alias for it similar to the standalone CMake approach. In GRPC_FOR_LOKI's extension CMakeLists.txt, minimal changes were made to that file mainly on the add_custom_command, target_include_directories and target_link_libraries commands. You'll see for Grpc.cmake for conan approach, I created similar variables to standalone CMake approach for GRPC_CPP_PLUGIN, INCLUDE_DIR, PROTOBUF_INCLUDE_DIR, PROTOBUF_COMPILER getting the conan related properties. For the conan side, I also made improvements to the Python Maven Executable Wrapper script since I noticed there were times at the beginning when running maven version test, it didn't always run it successfully, so I updated conan maven executable path to be explicit for passing to Python script. I then moved the location of running maven version check into a CMake function and ran it after we included JavaMaven and JavaOpenJDK conan packages in the MiNiFi root CMakeLists.txt file. Thus, it succesfully runs maven version check at beginning each time. Its important to make sure we have conan maven to successfully build NiFi JNI and Java SFTP Test Server. Finally, on the standalone CMake approach for building PcapPlusPlus, I noticed some issues with PCAP headers not being found when building PcapPlusPlus, so I switched from execute_process to run the configure scripts to using ExternalProject_Add and I also added a bundle for LibPcap since the system LibPcap version wasn't always installed on the system. This problem with building PcapPlusPlus dependency on LibPcap didn't happen with my conan approach. It took a lot of time to fix the standalone CMake backward compatibility approach, but I did it for now since we're keeping support that approach. 99% of GTESTs ( 237/238) passed for conan.

UPDATE (May 11, 2024) - c96d430: Refactoring MiNiFi C++ build infrastructure adding Conan Packager support
Enabled JNI, SENSORS, USB_CAMERA, OPENCV, and SFTP, which required creating Maven 3.9.6 conan package updating the conan recipe to expose the Maven Executable environment variable in CMake and installing prebuilt Java OpenJDK 21.0.2 conan package and creating a python wrapper run_maven.py script for calling the conan maven Java build tool to successfully build Java SFTP Test Server for SFTP extension testing and maven to build Apache NiFi 1.9.0 JNI Assembly and Framework; We had to go the python maven wrapper script approach since I ran into some issues calling conan maven executable from CMake directly; Also added prebuilt install for libssh2 conan package toward SFTP extensions; Thus, now the Java support for MiNiFi C++ is less reliant on the user’s system since we can handle building this part of MiNiFi C++ using conan; Also created an OpenCV 4.8.1 conan package updating the conan recipe to use MiNiFi C++ conan package dependencies from libtiff, ffmpeg and xz_utils (liblzma) and successfully build MiNiFi OpenCV extension; Also installed prebuilt libuvc conan package for enabling USB_CAMERA. For the conan build approach, 99% (235/236) of GTESTs passed. For the standalone CMake build approach, 99% (234/236) of GTESTs passed.

  • Also we could look at updating Apache NiFi 1.9.0 to latest 2.0 version. I noticed we use a Java SFTP Test Server now that NiFi has native python custom extensibility, maybe we could look at Python SFTP Test Server?

UPDATE (May 11, 2024) - 64dd8ad: Enabled Build LUA SCRIPTING, ENCRYPT CONFIG, SPLUNK, ELASTIC SEARCH, TEST PROCESSORS, GRANFANA LOKI and CONTROLLER by Conan & CMake. For building these components with conan, the LUA SCRIPTING extension required me updating its Lua.cmake and Sol2.cmake files, so we can use the conan packages in CMake when USE_CONAN_PACKAGE condition is true. For lua and sol2 conan packages, I was able to just install the prebuilt binary conan packages from conancenter directly since conancenter provided the versions that match with MiNiFi C++'s standalone CMake approach's lua 5.4.6 and sol2 3.3.0 source built versions. MiNiFi controller being enabled required updating the ArgParse.cmake file, so we could also account for the condition when we build the controller using conan, thus I added back using the argparse conan package. After enabling these components, I was able to successfully build them for MiNiFi using conan. I also verified backward compatibility for building these components in standalone CMake worked too. We achieved similar results for GTESTs where out of 230 tests, 99% of them passed for conan build and 100% passed for standalone CMake build.

UPDATE (May 11, 2024) - f5cfb79: Enabled Build MQTT & PCAP MiNiFi PCAP Extension by Conan & CMake. Refactored the pcap extension CMake code moving the part that downloads and builds pcapplusplus over to its own PcapPlusPlus.cmake file. Also added conan build condition to that .cmake file, so we can build PcapPlusPlus using conan while still keeping backward compatibility using standalone CMake approach. While I was able to build MiNiFi C++ with PCAP Extension enabled using standalone CMake approach, when I built it using conan build approach, I found pcap extension complaining about libminifi core headers and realized include_directories(include) didnt share all the header directory paths from libminifi to pcap extension. Thus, I created a python script under libminifi/cmake/python to parse for the libminifi header directories generating a include_dirs.cmake file with the LIBMINIFI_INCLUDE_DIRS CMake variable list that gets stored in build folder and followed by libminifi/include_dirs.cmake. That way we keep our source libminifi directory and its subdirectories clean. Overall, I was able to build MiNiFi C++ with MQTT and PCAP extensions enabled using conan & cmake. conan build achieved 99% (222/223) of GTESTs passing. standalone cmake build achieved 100% of GTESTs passing.

UPDATE (May 9, 2024) - 63290db: Refactored IODBC and SOCI libraries, so they are now CMake Bundles for easier integration with Conan. Thus, minifi-sql library now calls include(BundledIODBC), use_bundled_iodbc(...) and include(BundledSOCI) and use_bundled_soci(...). Verified I can still build MiNiFi C++ using standalone CMake. 100% (220/220) of GTESTS passed using standalone CMake. Integrated conan for building SOCI & ODBC for MiNiFi SQL extension. 4232399 Added a SOCI conan recipe to create SOCI conan package with unixODBC conan package, so SOCI comes with targets SOCI::libsoci_core and SOCI::lib;soci_odbc. Verified I can build MiNiFi C++'s minifi-sql extension using conan build. However, with the conan build approach for minifi-sql extension, I was only able to build minifi-sql as a STATIC library succesfully. When I tried building minifi-sql as a SHARED library, I ran into multiple undefined references to SOCI functions. I think to fix the minifi-sql SHARED lib with conan will require making further updates to the SOCI conanfile.py's package_info() method. After building MiNiFi with conan including minifi-sql, I ran GTESTs and 99% of tests passed. I will look further into minifi-sql SHARED library to bring that approach later. For now, at least we can build minifi-sql as a STATIC library.

UPDATE (May 9, 2024) - ca3d6df: Enabled MiNiFi C++ extensions GPS and COAP. I added conan recipe for libcoap keeping the windows cmake patches for minifi. Since the original libcoap conan recipe's conanfile.py on conan-center-index was tailored for CMake building libcoap versions > 4.3.0, I modified the recipe, so we can build libcoap 4.2.1, which didnt have CMake support yet and thus, I updated that conanfile.py to run 'autogen.sh' to create the configure script, I passed the configure args to the configure script and then I used conan's Autotools() to run the configure script and then ran the generated make file to build libcoap 4.2.1. I updated BundledLibCoAP to account for the conan way, but kept backward compatibility for standalone CMake way. For a later update when I test on Windows, I need to add support into the libcoap 4.2.1 conanfile.py for adding and applying the windows cmake patch to build libcoap. For now, I am mainly testing this new conan build infra for MiNiFi on Ubuntu 22.04 docker container and backward compatibility for standalone CMake on Ubuntu too. At least 99% of GTESTs passed (215/216) on conan build and 100% on standalone CMake build.

UPDATE (May 8, 2024) - 93fe5d1: Enabled NANOFI lib and binary executable, Enabled MiNiFi C++ extensions SYSTEMD, PROCFS, LIBARCHIVE and XY_UTILS (LZMA). I added conan recipe for liblzma (xy_utils) and libarchive keeping the patches for minifi. One area I found an issue with was integrating openssl conan package with libarchive conan package and libarchive complained about not being able to find openssl::openssl target. I did try to solve the issue by updating the libarchive conan recipe, but wasn't able to solve that openssl integration issue. I think the problem comes from libarchive CMakeLists.txt and its integration with conan in the repo. I opened an issue on conan-center-index to help with resolving this issue in case I get around to solving it in time. Good news is that I was still able to create libarchive conan package with the configuration settings similar to how we do it the standalone CMake BundledLibArchive.cmake way just in libarchive conan recipe. I also verified I can build these minifi extensions (LibArchive, LZMA, Procfs, Systemd) and NANOFI using conan build and standalone CMake. The standalone CMake way still allows us to build LibArchive with OpenSSL enabled. 213/214 (99%) GTESTs passed after building MiNiFi C++ with conan.

UPDATE (May 7, 2024) - 2460e84: Enabled OPS and OPC MiNiFi Options, so those MiNiFi C++ extensions are built using Conan CMake and their bundled CMake dependencies use their particular open62541/1.3.3@minifi/dev and mbedtls/2.16.3@minifi/dev conan packages. I had to fix the mbedtls conan package from conan center index since it wasnt compatible when open62541 tried to integrate with mbedtls. In the thirdparty folder, you will see on line 139 where I updated libembedtls to be libmbedtls and fixing that typo allowed open62541 conan package to be able to be created with mbedtls as the encryption. I plan to make a follow up PR on conan center index for mbedtls conan recipe. Currently, we are able to build MiNiFi's additional extensions for OPS and OPC using conan. Verified backward compatibility with standalone CMake works, so I can also just build MiNiFi using CMake only.

P1: Verified Can Build MiNiFi Core, MainExe, Standard-Processors, GTESTs with Conan CMake

UPDATE (May 5, 2024) - 446ddb6: Built MiNiFi C++ GTESTs for MiNiFi Core, MainExe and Standard-Processors and the other relevant that needed to be activated for these 3 essential components, I also set SKIP_TESTS to OFF, I set ENABLED_EXPRESSION_LANGUAGE to ON, ENABLE_BZIP2 to ON, BUILD_ROCKSDB to ON, ENABLE_ROCKSDB to ON to build GTESTs using conan build. I updated Bundle Rocksdb, Find Rocksdb and Bundle BZip2 to account for conan approach while keeping backward compatibility with standalone CMake approach. When running MiNiFi C++ GTESTs for both conan build and standalone CMake build approaches, 99% of tests passed.

UPDATE (May 4, 2024) - 7eb8beb: Verified I can build MiNiFi C++ with standalone CMake like how we currently do it even after integrating conan into the build infrastructure. CMake built MiNiFi Core, Main and Standard Processors successfully. Also double checked that conan build MiNiFi still works.

UPDATE (May 4, 2024) - 2d1b884: Updated MiNiFi CPP code with Conan & CMake to build libMiNiFi core-minifi and MiNiFi Main's minifiexe and minifi-standard-processors extension with minimal set of external libraries needed. If you look at our conanfile.py, you'll see minifi_core_external_libraries, which is a tuple of the minimal external libraries needed to build these two essential building blocks of MiNFi CPP. Next when you look at conanfile.py MiNiFiCppMain class's generate(self) method, you'll see I have enabled USE_CONAN_PACKAGER to ON, SKIP_TESTS to ON, and then the essential external library dependencies that aren needed for the core-minifi library and minifiexe binary executable, which are ENABLE_OPENWSMAN, ENABLE_OPENSSL, ENABLE_CIVET, ENABLE_CURL. I notice most of these external libraries are in the extensions folder and add some dependencies on these custom extensions that are created. I think we should consider creating an extensions subfolder for core-minifi related extensions.

I have included a sample of conan integration providing a faster alternative to CMake's FetchContent for building MiNiFi CPP faster first using abseil external lib dependency managed by conan. I will update the rest of MiNiFi CPP's FetchContent with conditional statements for including conan packager's prebuilt binary C++ conan package for MiNiFi.

Also kept backward compatibility for cmake FetchContent by adding two MiNiFi CPP options: one for using CMake FetchContent and the other for using the more faster approach conan packager to manage C++ external lib dependencies. With conan, usually conan will install prebuilt binary conan packages from conancenter or some other conan repo where the prebuilt binary conan packages reside. For now, we'll stick with using conancenter as it is open source.

Verified I can run conan install command from MiNiFi root folder where conanfile.py resides and successfully install abseil prebuilt binary conan package for MiNiFi C++, then ran cmake generate from MiNiFi's build folder and then started to run make. Stopped early on make since MiNiFi would try to download and build all the remaining external lib dependencies with CMake FetchContent since they arent managed by conan yet.

Also in Abseil.cmake's file, I added conditional statement to check if user set the USE_CONAN_PACKAGER option or the USE_CMAKE_FETCH_CONTENT option. Will do similar approach for the remaining external lib dependencies

Here are the steps I use to build MiNiFi C++ with Conan & CMake:

Build MiNiFi C++ with Conan & CMake

Install MiNiFi C++ External Lib Deps with Conan

# conanfile.py is in root dir of MiNiFi C++ project
cd nifi-minifi-cpp

# install conan packages for MiNiFi C++ from dir conanfile.py is located, build the ones that aren't available on conancenter
# conan install . --build=missing --output-folder=build -pr=$HOME/src/james/pipeline/nifi-minifi-cpp/etc/build/conan/profiles/release-linux

# install conan packages for MiNiFi C++ from dir's conanfile.py, fails if prebuilt binary conan package not on conancenter
# or not on your remote conan repo
conan install . --output-folder=build -pr=$HOME/src/james/pipeline/nifi-minifi-cpp/etc/build/conan/profiles/release-linux

Build MiNiFi C++ Project with Conan CMake

# install conan packages and build MiNiFi C++ using Conan & CMake, we run this way, so conan's tc.cache_variables
# are like passing -D{minifi_option} upon running cmake generate, then make to build MiNiFi C++
conan build . --output-folder=build -pr=/home/bizon/src/jam-repo/main/etc/build/conan/profiles/release-linux

Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically main)?

  • Is your initial contribution a single, squashed commit?

For code changes:

  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file?
  • If applicable, have you updated the NOTICE file?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.

Also kept backward compatibility for cmake FetchContent by adding two MiNiFi CPP options: one
for using CMake FetchContent and the other for using the more faster approach conan packager
to manage C++ external lib dependencies. With conan, usually conan will install prebuilt binary
conan packages from conancenter or some other conan repo where the prebuilt binary conan packages
reside. For now, we'll stick with using conancenter as it is open source.

Verified I can run conan install command from MiNiFi root folder where conanfile.py resides
and successfully install abseil prebuilt binary conan package for MiNiFi C++, then ran cmake
generate from MiNiFi's build folder and then started to run make. Stopped early on make
since MiNiFi would try to download and build all the remaining external lib dependencies with
CMake FetchContent since they arent managed by conan yet.

Also in Abseil.cmake's file, I added conditional statement to check if user set the USE_CONAN_PACKAGER
option or the USE_CMAKE_FETCH_CONTENT option. Will do similar approach for the remaining external lib
dependencies
@james94 james94 changed the title MINIFICPP-2346: Switch to C++ Conan Packager to Replace CMake FetchContent for Faster MiNiFi CPP Builds MINIFICPP-2346: Replace CMake FetchContent with Conan Packager for Faster MiNiFi CPP Builds May 2, 2024
@james94 james94 changed the title MINIFICPP-2346: Replace CMake FetchContent with Conan Packager for Faster MiNiFi CPP Builds MINIFICPP-2346: Replace CMake FetchContent with Conan Packager for Faster Builds May 2, 2024
Still stopped on make early as I am working toward adding each MiNiFi CPP external lib dependency
to be managed by conan packager for installing. conan install and cmake generate worked fine
@lordgamez
Copy link
Contributor

Hi @james94, thanks for the heads up, this is great stuff you have here! Last week we actually talked about having this feature implemented as part of the 1.0.0 release, so it is awesome you've already started working on this.

For the first iteration I would probably stick to having the old behaviour as the default and have a separate CI job with the USE_CONAN_PACKAGER variable enabled to test the build using Conan thirdparty packages. If the USE_CONAN_PACKAGER is OFF then everything would be built as it is at the moment, so probably the USE_CMAKE_FETCH_CONTENT wouldn't be needed.

But these are just small details that can be discussed on the review after the PR is ready, can't wait to see more of it!

@szaszm
Copy link
Member

szaszm commented May 2, 2024

There are a few things we would need for this to be viable:

  • The python bootstrap should be updated to support selecting each extension with its dependencies installed from Conan vs FetchContent
  • I think CMake should be updated to allow selecting the dependency source on a per-extension basis. This is also needed to implement the first point.
  • (optional) If we're doing the per-extension dependency source selection, a normal find_package could be added as well, to use the system version of libs. This may not be trivial to get working, but if it works at least at a few places, it can be an easy win.
  • I wouldn't flood FetchContent users with "SLOW" messages. Until we can ensure that all dependencies have viable binary packages on conan center, there isn't necessarily a build speed difference. We need to discuss how to handle these cases with Conan. And even after we can ensure binary packages for all major targets, FetchContent can still be a viable option, even if it builds the packages.

Let's see how this progresses, these can be done separately as well.

@james94
Copy link
Contributor Author

james94 commented May 2, 2024

@lordgamez thanks for the suggestion, I'll switch the default option to USE_CMAKE_FETCH_CONTENT for now. I'm excited to hear it was planned for 1.0.0 release to bring this feature to the build infrastructure. When is the planned release date, so I can track that target date?

@james94
Copy link
Contributor Author

james94 commented May 2, 2024

@szaszm thanks for sharing the requirements and what you guys are looking for to make this PR viable. I will shift the updates to meet those requirements. I appreciate it.

@szaszm
Copy link
Member

szaszm commented May 2, 2024

1.0.0 will be released some time after NiFi 2.0, but there is no clear target date. We were just discussing the idea (so no commitment), as third party library builds are already taking up the majority of the build time. I think someone would have given it a second try in the following months.
I did a first try of moving everything to Conan 2-3 years ago, but it quickly got too complex. Me not being very familiar with Conan probably didn't help it.
Thanks for the contribution! :)

… Main, Standard Processors

Verified I can run conan install and conan build to install minimal external library conan packages needed
by libMiNiFi, MiNiFi Main and MiNiFi Standard Processors followed by building the libMiNiFi's core-minifi
library, MiNiFi Main's minifiexe binary executable and the minifi-standard-processor extension.

If you look at our conanfile.py, you'll see minifi_core_external_libraries tuple, which represents the
minimal external libraries needed to build these two essential building blocks of MiNiFi CPP followed
by the minifi-standard-processors extension.

Next when you look at conanfile.py MiNiFiCppMain class's generate(self) method, you'll see I have enabled
USE_CONAN_PACKAGE to ON, SKIP_TESTS to ON, and the essential external library dependencies that are needed
for the core-minifi library, minifiexe binary executable and the minifi-standard-processors extension,
which are ENABLE_OPENWSMAN, ENABLE_OPENSSL, ENABLE_CIVET, ENABLE_CURL.

Feel free to review Jira MINIFICPP-2346 for more information. I also include a snippet of the terminal
showing the MiNiFi CPP code built successfully
… Conan

Previously, I tested building MiNiFi with conan. I needed to verify I could
also build MiNiFi with standalone CMake until it successfully built
@james94 james94 changed the title MINIFICPP-2346: Replace CMake FetchContent with Conan Packager for Faster Builds MINIFICPP-2346: Build MiNiFi Core, MainExe, Standard-Processors with Conan Packager May 5, 2024
@james94 james94 changed the title MINIFICPP-2346: Build MiNiFi Core, MainExe, Standard-Processors with Conan Packager MINIFICPP-2346: Build MiNiFi Core, MainExe, Standard-Processors with Conan2 May 5, 2024
james94 added 10 commits May 5, 2024 21:58
…onan & CMake

Additionally, those 3 essential components of MiNiFi C++ also have dependencies,
which I made sure I could install using both approaches of conan & standalone CMake.
Those dependencies for MiNiFi also have GTESTs that were ran along with those 3 essential
components of MiNiFi C++.

In both cases of building MiNiFi C++ GTests using conan & standalone CMake, I was able
to achieve 99% of tests passing.

For more info refer to Jira MINIFICPP-2346, where I include more info under 'UPDATE (May 5, 2024)'
followed by this commit hash being included in that section
Thus, those MiNiFi C++ extensions are built and their dependencies use their particular
open6254/1.3.3@minifi/dev and mbedtls/2.16.3@minifi/dev conan packages when building
MiNiFi with conan. Also I had to fix the mbedtls conan package from conan center index
since it wasnt compatible when open6254 tried to integrate with mbedtls, open62541 conan
package upon being created complained about not being able to find some mbedtls conan package
headers. After that fix to mbedtls, we are able to create open62541 and mbedtls conan packages
for MiNiFi C++ with no issues. Also I add on the user 'minifi' and channel 'dev' to signify that
there were particular updates that needed to be made for creating these two conan packages for
MiNiFi C++. However, the fix that I applied to mbedtls conan package can be contributed later
to conan center index repo since its more of a general fix for mbedtls integration with open62541,
which I plan to do later. Currently with this commit, we are able to build MiNiFi's additional
extensions for OPS and OPC using conan. I also verified the backward compatibility by building
MiNiFi with these 2 additional extensions using standalone CMake and it built successfully.
I ran GTESTs with 99% of tests passing too
…& CMake

Updated MiNiFi C++ to be able to build NANOFI lib(s) and binary executables, build MiNIFi C++
extensions Systemd, Procfs, Libarchive and Lzma using conan and then verified backward
compatibility with CMake still works. I added concan recipes for liblzma (xy_utils) and
libarchive and openssl. One area I found an issue with was integrating openssl conan package
with libarchive conan package where libarchive complained about not being able to find openssl::openssl,
I did try solving that libarchive integration openssl issue for the conan package, but wasn't
able to figure it out yet. I think the problem comes from libarchive CMakeLists.txt and its integration
conan in the repo. I opened an issue on conan-center-index to help with resolving this issue in case
I do solve it in time, I do plan to contribute the libarchive openssl conan package integration
fix to conan-center-index repo.

Good news is I was able to create the libarchive conan package with the configuration settings similar
to how we do it in the standalone CMake BundleLibArchive.cmake file. I also verified I can build these
minifi extensions (LibArchive, LZMA, Procfs, Systemd) and NANOFI using conan build and standalone CMake.
The standalone CMake way still allows us to build LibArchive with OpenSSL enabled. 213/214 (99%) GTESTs
passed after building MiNiFi C++ with conan.
I added conan recipe for libcoap keeping the windows cmake patches for minifi.
Since the original libcoap conan recipe's conanfile.py on conan-center-index was tailored for CMake building libcoap versions > 4.3.0,
I modified the recipe, so we can build libcoap 4.2.1, which didnt have CMake support yet and thus,
I updated that conanfile.py to run 'autogen.sh' to create the configure script,
I passed the configure args to the configure script and then I used conan's Autotools() to run the configure script and then ran the generated make file to build libcoap 4.2.1.
I updated BundledLibCoAP to account for the conan way, but kept backward compatibility for standalone CMake way.
For a later update when I test on Windows, I need to add support into the libcoap 4.2.1 conanfile.py for adding
and applying the windows cmake patch to build libcoap.

For now, I am mainly testing this new conan build infra for MiNiFi on Ubuntu 22.04 docker container
and backward compatibility for standalone CMake on Ubuntu too. 99% of GTESTs passed (215/216).
…an Integ

Thus, minifi-sql library now calls include(BundledIODBC), use_bundled_iodbc(...), include(BundledSOCI)
and use_bundled_soci(...). Verified I can still build MiNiFi C++ using standalone CMake.
100% (220/220) of GTESTs passed using standalone CMake.
Added a SOCI conan recipe to create SOCI conan package with unixODBC conan package
, so SOCI comes with targets SOCI::libsoci_core and SOCI::libsoci_odbc. I verified
I can build MiNiFi C++'s minifi-sql extension using conan build. However, with the
conan build minifi-sql approach, I was only able to build minifi-sql as STATIC library
successfully. When I tried building minifi-sql as a SHARED library, I ran into multiple
undefined references to SOCI functions. I think to fix the minifi-sql SHARED lib with
conan will require making further updates to SOCI conanfile.py's package_info() method.

After building MiNiFi with conan including minifi-sql, I ran GTESTs and 99% of tests passed.

I will look into minifi-sql SHARED library to bring back that approach later. For now,
at least we can build minifi-sql as a STATIC library.
Refactored the pcap extension CMake code moving the part that downloads and builds
pcapplusplus over to its own PcapPlusPlus.cmake file. Also added conan build condition
to that .cmake file, so we can build PcapPlusPlus using conan while still keeping
backward compatibility using standalone CMake approach.

While I was able to build MiNiFi C++ with PCAP Extension enabled using standalone CMake
approach, when I built it using conan build approach, I found pcap extension complaining
about libminifi core headers and realized include_directories(include) didnt share
all the header directory paths from libminifi to pcap extension. Thus, I created a python
script under libminifi/cmake/python to parse for the libminifi header directories generating
a include_dirs.cmake file with the LIBMINIFI_INCLUDE_DIRS CMake variable list that gets
stored in build folder and followed by libminifi/include_dirs.cmake. That way we keep our source
libminifi directory and its subdirectories clean.

Overall, I was able to build MiNiFi C++ with MQTT and PCAP extensions enabled using conan & cmake.
conan build achieved 99% (222/223) of GTESTs passing. standalone cmake build achieved 100% of
GTESTs passing
…LASTIC SEARCH, TEST PROCESSORS, GRANFANA LOKI, CONTROLLER

For building these components with conan, the LUA SCRIPTING extension required me updating its Lua.cmake and Sol2.cmake files,
so we can use the conan packages in CMake when USE_CONAN_PACKAGE condition is true. For lua and sol2 conan packages,
I was able to just install the prebuilt binary conan packages from conancenter directly since conancenter provided the versions that match
with MiNiFi C++'s standalone CMake approach's lua 5.4.6 and sol2 3.3.0 source built versions. MiNiFi controller being
enabled required updating the ArgParse.cmake file, so we could also account for the condition when we build the controller using conan,
thus I added back using the argparse conan package. After enabling these components, I was able to successfully build them
for MiNiFi using conan. I also verified backward compatibility for building these components in standalone CMake worked too.
We achieved similar results for GTESTs where out of 230 tests, 99% of them passed for conan build and 100% passed for standalone CMake build.
required creating Maven 3.9.6 conan package updating the conan recipe to expose the Maven Executable environment variable in CMake
and installing prebuilt Java OpenJDK 21.0.2 conan package and creating a python wrapper run_maven.py script
for calling the conan maven Java build tool to successfully build Java SFTP Test Server for SFTP extension testing
and maven to build Apache NiFi 1.9.0 JNI Assembly and Framework; We had to go the python maven wrapper script approach
since I ran into some issues calling conan maven executable from CMake directly; Also added prebuilt install for libssh2 conan package
toward SFTP extensions; Thus, now the Java support for MiNiFi C++ is less reliant on the user’s system since
we can handle building this part of MiNiFi C++ using conan; Also created an OpenCV 4.8.1 conan package updating the conan recipe
to use MiNiFi C++ conan package dependencies from libtiff, ffmpeg and xz_utils (liblzma) and successfully build MiNiFi OpenCV extension;
Also installed prebuilt libuvc conan package for enabling USB_CAMERA. For the conan build approach,
99% (235/236) of GTESTs passed. For the standalone CMake build approach, 99% (234/236) of GTESTs passed.

Also we could look at updating Apache NiFi 1.9.0 to latest 2.0 version. I noticed we use a Java SFTP Test Server now
that NiFi has native python custom extensibility, maybe we could look at Python SFTP Test Server?
… CMake

Improved Running Python Maven Executable for JNI & SFTP Test Server for conan approach and Improved standalone
CMake approach for building PcapPlusPlus with its dependency on LibPcap. LIBRDKAFKA required creating a librdkafka
conan package passing similar CMake definitions to it to build it like we did for standalone CMake approach. GRPC_FOR_LOKI
required creating a grpc conan package that uses minifi's openssl and libsystemd conan packages. In both LIBRDKAFKA &
GRPC_FOR_LOKI, I updated their .cmake Bundles to account for the condition of using their conan package, so minimal
changes would be needed to their extension CMakeLists.txt file. In LIBRDKAFKA's extension CMakeLists.txt file, it didnt really
change after integrating the conan package since I created a library alias for it similar to the standalone CMake approach. In
GRPC_FOR_LOKI's extension CMakeLists.txt, minimal changes were made to that file mainly on the add_custom_command,
target_include_directories and target_link_libraries commands. You'll see for Grpc.cmake for conan approach, I created similar
variables to standalone CMake approach for GRPC_CPP_PLUGIN, INCLUDE_DIR, PROTOBUF_INCLUDE_DIR,
PROTOBUF_COMPILER getting the conan related properties. For the conan side, I also made improvements to the Python
Maven Executable Wrapper script since I noticed there were times at the beginning when running maven version test, it didn't
always run it successfully, so I updated conan maven executable path to be explicit for passing to Python script. I then moved
the location of running maven version check into a CMake function and ran it after we included JavaMaven and JavaOpenJDK
conan packages in the MiNiFi root CMakeLists.txt file. Thus, it succesfully runs maven version check at beginning each time. Its
important to make sure we have conan maven to successfully build NiFi JNI and Java SFTP Test Server. Finally, on the
standalone CMake approach for building PcapPlusPlus, I noticed some issues with PCAP headers not being found when
building PcapPlusPlus, so I switched from execute_process to run the configure scripts to using ExternalProject_Add and I also
added a bundle for LibPcap since the system LibPcap version wasn't always installed on the system. This problem with building
PcapPlusPlus dependency on LibPcap didn't happen with my conan approach. It took a lot of time to fix the standalone CMake
backward compatibility approach, but I did it for now since we're keeping support that approach. 99% of GTESTs ( 237/238) passed for conan.
@martinzink
Copy link
Member

You will need to rebase your PR for the CI to work.

Also resolved conflicts, verified I can build MiNiFi using standalone CMake
and conan build approaches and ran GTESTs for both approaches getting at
least 99% GTESTs passing
MiNiFi 2346 Merge Latest Upstream Main Branch on 051324
@james94
Copy link
Contributor Author

james94 commented May 14, 2024

@martinzink thanks for the suggestion. I brought MINIFI-2346 to up to date with latest upstream MiNiFi C++ main branch. Ubuntu 20.04 and Docker build for integration tests passed. The other CI/CDs failed. Its probably related to some of the changes I am introducing to the build infrastructure. I have mainly been testing the updates in a Ubuntu 22.04 docker container. I haven't tested them yet on Windows or Mac OS X. Those 2 last OSs, I'd test later. I also would need to look into why CI/CD is failing too. After I finish refactoring build infrastructure to support Conan version 2 and keep backward compatibility with the standalone CMake approach, I will work toward updating the MiNiFi bootstrap py script with conan support too. I will get to the CI/CD issues as soon as I can, but those will probably be the last updates.

@szaszm
Copy link
Member

szaszm commented May 15, 2024

@james94 I'm busy with other commitments to keep up with this, but I'd like to prevent wasting too much of your time on something we can't use. We can't review a 17k diff, so it will never be merged as is stands now.

If you want to pursue this, please create a separate pull request, where you only convert a single dependency to build with conan, so we can provide feedback there, and only change what's absolutely necessary. A good first target could be OpenSSL, because it takes forever to build on Windows.

Please make an effort to keep the diff as small as possible, in the 100s if possible, but keep it under 1000. We can review many small pull requests, but not giants like this. Example: #603 didn't get any reviews, despite being a very valuable feature

As the conversion of the first dependency is reviewed, we both learn from it, and subsequent dependency source conversions will be easier. Please only pursue 1 or max 2 features at a time, so we can keep up and provide the necessary feedback. This will inevitably take a few months.

I'll also give some early feedback on things I noticed so far:

  • Please use a proper multi-choice option for selecting the source of a dependency instead of USE_CONAN_PACKAGER and USE_CMAKE_FETCH_CONTENT binary options. For example: MINIFI_OPENSSL_SOURCE: [conan|find_package|bundled]. The second option could keep just find_package, and rely on sources from the system. If it doesn't work, just having [conan|bundled] is fine.
  • Please don't include patches that are not strictly necessary to make conan builds work
  • Please prefix minifi-related options with MINIFI_ in cmake. Not all options follow this now, but future new CMake options should.
  • Please don't add new dependencies in the context of converting to Conan. For example, minifi doesn't depend on ffmpeg or libtiff at the moment. If they're needed for a new feature, the new feature needs to be a separate pull request.

@james94
Copy link
Contributor Author

james94 commented May 15, 2024

Thanks for your suggestion @szaszm

I was considering to break this PR into multiple parts because like you said there is a lot to review. Actually, with each major update and commit hash, my goal was to make it easier to track each section of the new update that I made. For example with just updating OpenSSL to have USE_CONAN_PACKAGER support and backward compatibility for USE_CMAKE_FETCH_CONTENT, which impacts libminifi, we could reference my current commit hash from UPDATE (May 4, 2024) - 2d1b884 and extract out just the part for OpenSSL, so its a smaller PR following your suggestions. Once its approved, we're on the same page for what you guys are looking for, we could look toward bringing up USE_CONAN_PACKAGER support for building the whole libminifi like I do in commit 2d1b884.

I will follow up with a smaller PR for OpenSSL impacting part of libminifi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants